fix(python): use resolving storage for python binding#2246
fix(python): use resolving storage for python binding#2246geruh merged 4 commits intoapache:mainfrom
Conversation
geruh
left a comment
There was a problem hiding this comment.
This looks like a great addition for the python side! Looks like the cargo file only brings in s3, fs, and memory here but the resolving factory lists more storage backends from opendal.
iceberg-rust/bindings/python/Cargo.toml
Line 36 in 1817c98
Good point! will fix it |
geruh
left a comment
There was a problem hiding this comment.
LGTM! All tests pass, built the binding and verified it works e2e with PyIceberg, and file scheme.
| .map_err(|e| PyRuntimeError::new_err(format!("Invalid table identifier: {e}")))?; | ||
|
|
||
| let factory = storage_factory_from_path(&metadata_location)?; | ||
| let factory = Arc::new(OpenDalResolvingStorageFactory::new()); |
There was a problem hiding this comment.
love it! its like java's ResolvingFileIO
There was a problem hiding this comment.
maybe we can start exporting this as another FileIO for pyiceberg
There was a problem hiding this comment.
totally agree that we should expose something like this to Pyiceberg
There is a tracking issue to expose storage factory as a configurable item to Pyiceberg: #2200
Which issue does this PR close?
What changes are included in this PR?
Are these changes tested?